npu-pack: import the .fconst MLP section (gate|up fused, then down_proj) by bong-water-water-bong · Pull Request #842 · 1bit-MONSTER/engine

bong-water-water-bong · 2026-11-02T19:09:51Z

What

The MLP section of an OGA pack's layer block — the last part of the pack the importer refused (72 projections on Qwen2.5-0.5B) — is now decoded, byte-verified and imported. After this, the only tensor 1bit npu-pack still refuses is model.norm (final), whose position is genuinely unestablished.

The layout, and why it is a measurement

Derived from the writer's own headers (mladfQuantMatrix / mladfWgtSubv in the vendor wheel's ops_common/mladf_matmul_matrix.hpp, concat_gate_up in matmulbias_weights_gateup.cpp), then checked against the bytes:

part	layout
`gate` + `up`	ONE fused `hidden x 2intermediate` tensor, rows interleaved in 32-column chunks (gate 32, up 32, …), in 128-column groups of 4,288-byte* subvolumes — the same struct at `Nsubv 64`: `[0,4096)` quants, `[4096,4160)` zero points, `[4160,4288)` 64 bf16 scales
—	one 64-byte parameter subvolume (`01 00 …`), the same bytes in every layer block
`down_proj`	`intermediate x hidden` in 64-column groups of the 2,176-byte subvolume the attention payload already uses

The three parts must fill the block stride exactly, and that identity — not a constant — decides whether a pack is one this decoder understands. On Qwen2.5-0.5B they end at 8,708,288 B, the measured stride, to the byte. Qwen3-1.7B and Qwen3-4B do not add up and refuse by name with the size they did add up to (their writer used different kernel shapes — the same runtime-shapes table engine#734 ran into).

Code

The same decoder at a second width. TileW gains rows (64 or 128 columns); the subvolume size, its zero/scale offsets and its quant index follow from it, so the attention payload, down_proj and the fused width share one verified code path. gu_half() splits a fused 128-column group into the two projections (gate = rows 0..31 and 64..95, up = 32..63 and 96..127).

Verified on the shipped pack

2160/2160 MLP groups decode and re-encode to the pack's exact bytes (24 layers × 76 gate|up units + 24 × 14 down groups) — the same round-trip contract the attention tiles are held to.
Every weight in them agrees byte for byte with an independent decode of the same layer: the decode program stores gate/up fused at Nsubv 64 while the prefill program stores them separately at Nsubv 32, and the two agree on codes, scales and zero points for all 24 layers × {gate, up, down}.
New CI fixture tests/golden/oga_mlp_gu_group.bin — one 68,608-byte unit copied verbatim from the pack by tools/oga_mlp_gu_golden.py, plus the container tiles its two projections give — so the decode and the split are checked against the vendor writer's own bytes where no vendor stack exists, with two controls that must fail (a moved code nibble, a one-ulp scale).
Container: 290 tensors / 67,088 q4_1 tiles (was 218 / 25,136), and the engine's own Model opens it.

The bf16 parameter bound is unchanged and reported, not hidden: zero points are bf16-exact for 52.9 % of the MLP's 32-column groups (48.7 % of the attention ones), residual 3.34e-3 relative.

Checks run

On this branch, fresh Release/ninja/gcc-16 configure with ONEBIT_OGA_PACK:

tools/copyright.py --check → rc 0
ctest -R npu_pack_import → 2/2 (npu_pack_import 0.00 s incl. the new golden, npu_pack_import_oga 2.05 s on the real pack)
tools/oga_mlp_gu_golden.py <pack> --check → the fixtures re-derive from the pack

Not in this PR

model.norm (final) stays refused: its .fconst tail is [eps][pad][448 f32], not an RMSNorm weight. The MLP geometry for other hidden sizes is open (refused by name, with the arithmetic in the message). Nothing of the vendor's is linked, called or shipped — the reader is written from the documented format facts in amd-oss/npu-re/docs/FACTS-oga-pack-weight-layout.md §7.17.

github-actions · 2026-11-02T19:11:12Z

PR Reviewer Guide 🔍

(Review updated until commit `c4d8dfc`)

Here are some key observations to aid the review process:

🎫 Ticket compliance analysis 🔶 734 - Partially compliant Compliant requirements: The PR derives the `.fconst` layer-block stride and the two norm offsets from the pack's meta `N`/`K` instead of constants measured on one pack The PR verifies on the second family with the same epsilon-anchor check (48/48 on Qwen2.5-0.5B; 32 per bucket for Llama) The PR addresses the specific refusal of `model.layers.0.input_layernorm.weight` and `model.layers.0.post_attention_layernorm.weight` by generalizing the offset calculation Non-compliant requirements: The PR does not fully address the final norm refusal (`model.norm.weight`) as mentioned in the issue description Requires further human verification: Verification that the final norm refusal is indeed resolved after this PR (as it's mentioned as still being refused)
⏱️ Estimated effort to review: 4 🔵🔵🔵🔵⚪
🧪 PR contains tests
🔒 No security concerns identified
⚡ Recommended focus areas for review MLP Section Geometry Calculation The PR introduces a new `mlp_geom` function that calculates the MLP section geometry based on the pack's meta `N`/`K` values. This is a key change for generalizing the MLP section handling. However, the calculation relies on `intermediate` and `hidden` values being multiples of 64, which may not always be true for all model configurations. If these values are not multiples of 64, the function returns an error message, but it's unclear if this is handled gracefully in all cases. MlpGeom mlp_geom(int64_t hidden, int64_t inter, int64_t mlp_off, int64_t stride) { MlpGeom m; m.off = mlp_off; const int64_t gu_unit = group_bytes(hidden, kMlpRows); // 128 columns of K=hidden const int64_t down_group = group_bytes(inter, kTileRows); // 64 columns of K=intermediate if (inter <= 0) { m.why = "config.json gives no intermediate_size"; return m; } if (gu_unit == 0) { m.why = "hidden " + std::to_string(hidden) + " is not a positive multiple of 128, so the fused " "gate/up section is not 128-column groups of this shape"; return m; } if (down_group == 0) { m.why = "intermediate " + std::to_string(inter) + " is not a positive multiple of 128, so " "down_proj is not 64-column groups of this shape"; return m; } if (inter % kTileRows \|\| hidden % kTileRows) { m.why = "intermediate " + std::to_string(inter) + " / hidden " + std::to_string(hidden) + " are not multiples of 64, which the fused gate/up split needs"; return m; } m.gu_units = inter / kTileRows; m.down_groups = hidden / kTileRows; m.gu = m.gu_units * gu_unit; m.down = m.down_groups * down_group; m.total = m.off + m.gu + m.gap + m.down; if (m.total != stride) m.why = "the MLP section ends at " + std::to_string(m.total) + " B in a " + std::to_string(stride) + "-byte layer block, so its geometry is not the one this decoder was established on"; return m; } MLP Section Byte Verification The PR adds byte verification for the MLP section by decoding and re-encoding groups of tiles. This is a good safety measure, but the verification process relies on the `encode_tile` function which now supports variable `rows` parameter. If there are inconsistencies in how the encoding/decoding is handled for different row sizes, it could lead to false positives or negatives in the verification. // ── the MLP section: gate\|up fused, then down_proj ─────────────────────────────────── // Same contract as the attention payload: every group read here is decoded, re-encoded and // compared with the bytes it came from before any of it is written, and a group that does // not survive that is refused by name. The geometry is re-derived from this block rather // than taken from plan(), so the stride identity is checked against the file being read. if (p.mlp_gu_units) { const MlpGeom mg = mlp_geom(H, p.dims.intermediate, g.h1 + kNormOff + H * 2, g.stride); if (!mg.ok()) { late.push_back("the MLP section: " + mg.why); } else { const int inter = p.dims.intermediate; const int64_t gu_unit = group_bytes(H, kMlpRows); const int64_t down_group = group_bytes(inter, kTileRows); for (int L = 0; L < p.dims.layers; ++L) { const int64_t base_gu = int64_t(L) * g.stride + mg.off; const int64_t base_down = base_gu + mg.gu + mg.gap; std::vector<TileW> gate, up, down; bool gate_ok = true, up_ok = true, down_ok = true; bool gate_bias = false, up_bias = false, down_bias = false; for (int u = 0; u < int(mg.gu_units); ++u) { const uint8_t* src = at(fc, base_gu + int64_t(u) * gu_unit, gu_unit); const TileW grp = decode_tile(src, int(H), kMlpRows); ++r.mlp_groups_total; const std::vector<uint8_t> back = encode_tile(grp); if (back.size() == size_t(gu_unit) && std::memcmp(back.data(), src, size_t(gu_unit)) == 0) ++r.mlp_groups_ok; else gate_ok = up_ok = false; gate.push_back(gu_half(grp, false)); up.push_back(gu_half(grp, true)); } for (int gi = 0; gi < int(mg.down_groups); ++gi) { const uint8_t* src = at(fc, base_down + int64_t(gi) * down_group, down_group); const TileW grp = decode_tile(src, inter, kTileRows); ++r.mlp_groups_total; const std::vector<uint8_t> back = encode_tile(grp); if (back.size() == size_t(down_group) && std::memcmp(back.data(), src, size_t(down_group)) == 0) ++r.mlp_groups_ok; else down_ok = false; down.push_back(std::move(grp)); } auto emit = [&](const std::vector<TileW>& tiles, const char* suffix, bool exact, bool& bias_flag) { const std::string name = "model.layers." + std::to_string(L) + "." + suffix; for (const auto& t : tiles) for (uint16_t v : t.bias) bias_flag \|= (v != 0); if (!exact) { late.push_back(name + ".weight: the .fconst group does not survive " "decode->encode, so its fields are not understood"); return; } if (bias_flag) { late.push_back(name + ".weight: the MLP bias subvolumes are not empty and the " "container has no MLP bias"); return; } Q4 q = q_from_tiles(tiles); std::vector<uint8_t> tiled; long long zinexact = 0; tile_q4_1(q, tiled, &zinexact); mlp_zero_inexact += zinexact; mlp_zero_groups += (long long)q.scale.size(); const int64_t nt = int64_t(tiled.size() / kTileBytes); r.tiles += nt; w.add(name + ".weight", "I8", {nt, int64_t(kTileBytes)}, std::move(tiled)); ++r.tensors; ++r.mlp_projections; }; emit(gate, "mlp.gate_proj", gate_ok, gate_bias); emit(up, "mlp.up_proj", up_ok, up_bias); emit(down, "mlp.down_proj", down_ok, down_bias); } r.mlp_zero_inexact_pct = mlp_zero_groups ? 100.0 * double(mlp_zero_inexact) / double(mlp_zero_groups) : 0.0; } } MLP Golden Test Coverage The PR adds a test case for MLP golden data, which is good for ensuring correctness. However, the test only covers one specific case (K=896) and doesn't verify the generalization for other values. It's important to ensure that the test covers a range of values to validate that the generalization works correctly. // ── 3c. the .fconst gate\|up unit against shipped pack bytes ───────────────── void test_mlp_gu(const std::string& golden_dir) { if (golden_dir.empty()) return; const std::string path = golden_dir + "/oga_mlp_gu_group.bin"; std::ifstream f(path, std::ios::binary); if (!f) { std::printf("skip MLP golden: %s absent\n", path.c_str()); return; } std::vector<uint8_t> group((std::istreambuf_iterator<char>(f)), std::istreambuf_iterator<char>()); const int k = 896; const int64_t want = pi::group_bytes(k, 128); check(int64_t(group.size()) == want, "MLP golden: the fixture is one " + std::to_string(want) + "-byte gate\|up unit of K=" + std::to_string(k) + " (got " + std::to_string(group.size()) + ")"); if (int64_t(group.size()) != want) return; const pi::TileW g = pi::decode_tile(group.data(), k, 128); check(g.rows == 128 && g.code.size() == 128u * size_t(k), "MLP golden: the unit decodes to 128 columns"); const std::vector<uint8_t> back = pi::encode_tile(g); check(back == group, "MLP golden: encode(decode(unit)) reproduces the pack's bytes exactly"); if (back != group) for (size_t i = 0; i < std::min(back.size(), group.size()); ++i) if (back[i] != group[i]) { std::fprintf(stderr, " first difference at byte %zu: ours %02x, pack %02x\n", i, back[i], group[i]); break; } { pi::TileW bad = g; bad.code[70] ^= 0x01; check(pi::encode_tile(bad) != group, "MLP golden: a single changed code nibble is caught"); pi::TileW bad2 = g; bad2.scale[100] = 1.004f; check(pi::encode_tile(bad2) != group, "MLP golden: a one-ulp scale change is caught"); } // engine#849's class at the MLP width: the round trip above cannot see the zero-point pairing (it is // invariant under any permutation applied on both sides), and the fused unit's 4,288-byte subvolume // carries 64 rows per half instead of 32, so the same two checks are made here at `rows = 128`. { const int kbs = k / 128; const int64_t subv = pi::group_bytes(k, 128) / (2 kbs + 2); const int subv_rows = 64; // = rows / 2 for the fused width std::vector<uint8_t> raw(size_t(pi::group_bytes(k, 128)), 0); for (int h = 0; h < 2; ++h) for (int kb = 0; kb < kbs; ++kb) for (int i = 0; i < subv_rows / 2; ++i) { // a nibble holds 0..15, and the fused width uses 32 of these bytes per half, so the // pattern repeats: what matters is which row each nibble lands on, not that the 32 // bytes are distinct. const int v = i & 0x0F; raw[size_t((2 + 2 * kb + h) * subv + 64 * subv_rows + i)] = uint8_t(v \| ((15 - v) << 4)); } const pi::TileW z = pi::decode_tile(raw.data(), k, 128); bool pairs_even_odd = true; for (int h = 0; h < 2 && pairs_even_odd; ++h) for (int kb = 0; kb < kbs && pairs_even_odd; ++kb) for (int i = 0; i < subv_rows / 2; ++i) { const uint8_t v = uint8_t(i & 0x0F); pairs_even_odd = pairs_even_odd && z.zp[size_t(h * subv_rows + 2 * i) * size_t(kbs) + size_t(kb)] == v && z.zp[size_t(h * subv_rows + 2 * i + 1) * size_t(kbs) + size_t(kb)] == uint8_t(15 - v); } check(pairs_even_odd, "MLP golden: zero-point nibbles pair with rows 2i / 2i+1 (even/odd) at 128 rows"); // and on the pack's own unit: a correctly paired zero point sits where its 128 codes cluster double n = 0, sz = 0, sm = 0, szz = 0, smm = 0, szm = 0; for (int r = 0; r < 128; ++r) for (int b = 0; b < kbs; ++b) { double sum = 0; for (int c = 0; c < 128; ++c) sum += g.code[size_t(r) * size_t(k) + size_t(b * 128 + c)]; const double m = sum / 128.0, zz = double(g.zp[size_t(r) * size_t(kbs) + size_t(b)]); n += 1; sz += zz; sm += m; szz += zz * zz; smm += m * m; szm += zz * m; } const double cov = szm / n - (sz / n) * (sm / n); const double sd = std::sqrt(std::max(0.0, szz / n - (sz / n) * (sz / n))); const double sm2 = std::sqrt(std::max(0.0, smm / n - (sm / n) * (sm / n))); const double corr = (sd > 0 && sm2 > 0) ? cov / (sd * sm2) : 0.0; char buf[160]; std::snprintf(buf, sizeof buf, "MLP golden: the zero points sit under their own codes (corr %+0.4f, want > 0.9)", corr); check(corr > 0.9, buf); } // the fused unit holds gate and up interleaved in 32-column chunks: both halves have to reproduce the // container tiles recorded from an independent decode of the same bytes std::ifstream qf(golden_dir + "/oga_mlp_gu_q4_1.bin", std::ios::binary); std::vector<uint8_t> golden((std::istreambuf_iterator<char>(qf)), std::istreambuf_iterator<char>()); std::vector<uint8_t> tiled; for (const bool up : {false, true}) { const pi::Q4 q = pi::q_from_tiles({pi::gu_half(g, up)}); std::vector<uint8_t> part; pi::tile_q4_1(q, part); tiled.insert(tiled.end(), part.begin(), part.end()); } check(golden.size() == tiled.size(), "MLP golden: gate and up give " + std::to_string(tiled.size() / onebit::npu::kTileBytes) + " container tiles of 64 rows each"); check(golden == tiled, "MLP golden: container tiles byte-identical to the recorded ones"); if (golden != tiled) for (size_t i = 0; i < std::min(golden.size(), tiled.size()); ++i) if (golden[i] != tiled[i]) { std::fprintf(stderr, " first tile difference at byte %zu: golden %02x, ours %02x\n", i, golden[i], tiled[i]); break; } // the split has to be able to fail: swapping the two halves of one column-block must change the tiles { pi::TileW swapped = g; const size_t kb = size_t(k); for (int r = 0; r < 32; ++r) { std::swap_ranges(&swapped.code[size_t(r) * kb], &swapped.code[size_t(r) * kb] + kb, &swapped.code[size_t(32 + r) * kb]); } std::vector<uint8_t> other; // gate AND up, as `tiled` is: a gate-only stream differs in length for (const bool up : {false, true}) { std::vector<uint8_t> part; pi::tile_q4_1(pi::q_from_tiles({pi::gu_half(swapped, up)}), part); other.insert(other.end(), part.begin(), part.end()); } check(other.size() == tiled.size() && other != tiled, "MLP golden: a gate/up swap inside a column block is caught"); } std::printf("MLP golden: 1 unit, encode/decode exact, %zu container tiles identical\n", tiled.size() / onebit::npu::kTileBytes); }

bong-water-water-bong

VERDICT: CHANGES at c84110ae — one item, and it is a one-line cast: the aggregate CodeQL check is red on a new
alert in the file this PR adds. Everything else here reproduces on the shipped pack, including the parts I could check
independently of the importer's own code.

The blocker. CodeQL alert #70, rule cpp/integer-multiplication-cast-to-long (high),
tests/npu_pack_import_test.cpp:657:

check(r.mlp_groups_total == layers * (p.dims.intermediate / 64 + p.dims.hidden / 64),

r.mlp_groups_total is long long but layers is int, so the multiply happens in int and is converted afterwards
(the values here are 24 x 90, so nothing overflows — the query is right about the form, not about this pack).
Fix: int64_t(layers) * (...). #844 carries the same line, so fixing it in this branch fixes both; I re-verdict
both the moment a head moves.

Built and run here (fresh Release/ninja/gcc-16 configure, -DONEBIT_OGA_PACK=<the 0.5B pack>):
ninja npu_pack_import_test onebit rc 0, the only warning the pre-existing nlohmann one in app/serve.cpp;
ctest -R npu_pack_import -> 2/2 (npu_pack_import 0.04 s, npu_pack_import_oga 6.89 s). My own import of the
shipped Qwen2.5-0.5B-Instruct_rai_1.8.0_npu_4K:

imported 290 tensors, 67088 q4_1 tiles
  self-check: lm_head vs the pack's fp16 embedding  corr 0.999999  rel_err 0.000700
  controls:   nibble-swapped corr -0.0008, no zero-point corr 0.8788 (both must fail)
  attention:  768/768 .fconst tiles survive decode->encode byte for byte; 96 projections + 72 bias tensors
  MLP:        2160/2160 .fconst groups survive decode->encode byte for byte; 72 projections imported
  fconst:     stride 8708288, second header 1115968 (measured from the eps anchors); 48/48 RMSNorm anchors found
  incomplete: 1 tensors

so 290 / 67,088 / 2160-of-2160 / one refusal are all the PR's numbers, and the negative controls fire.

Independent of pack_import.cpp, I re-derived the load-bearing claims in Python:

tests/golden/oga_mlp_gu_group.bin is byte-identical to the pack's bytes at offset 1,117,824, and that offset is
exactly where the MLP section has to start: 1,115,968 (attention payload end) + 1,856 (norm envelope). Then
1,117,824 + 5,214,208 + 64 + 2,376,192 = 8,708,288 = the stride the epsilon anchors give on both files, so the
geometry identity really is an identity for this pack and not a fitted constant.
my own transcription of the 4,288-byte subvolume (§7.17: quants at (cp%4) + kk*4 + (cp/4)*512, 64 zero bytes at
4,096, 64 bf16 scales at 4,160, subvolume x2 + 2*(kblock+1), bias subvolumes 0 and 1) decodes that unit, splits
gate/up at the 32-column chunks, rebuilds the container tiles with the K -> 1,024 column padding and the bf16 zero
rounding, and gets 16 tiles byte-identical to oga_mlp_gu_q4_1.bin (sha256 422e84a0a3a8af54… on both sides).
Bias subvolumes are all zero, codes 0..15, scales 0.0030..0.0261, zero points 2..12 — plausible for a 4-bit MLP.
tile arithmetic: 24 layers x (608 gate + 608 up + 532 down) = 41,952 = 67,088 - 25,136, and 218 + 72 = 290.
the scope statement's refusals name the doc's own numbers on this box:
Qwen3-1.7B -> "the MLP section ends at 25,563,328 B in a 27,931,456-byte layer block" (2,368,128 unaccounted),
Qwen3-4B -> "49,562,816 B in 55,308,096" (5,745,280), both exactly FACTS §7.17. (Llama-3.2-1B refuses earlier,
for a pre-existing reason: fewer than 6 epsilon headers, so even the attention payload is not located — not this PR,
but worth knowing that pack is not in the "the MLP is the only gap" class.)

One thing the committed tests cannot catch, worth a follow-up rather than a gate item: a global gate/up swap
would pass everything here — the golden's split is itself the assumption, and the controls only catch a swap inside
one column block. The independent evidence for the assignment is the lane's prefill cross-decode (§7.17), and the
prefill .fconst that carries it ships in this same pack, so a CI test could compare the two physical layouts directly
(layer 0 alone would be enough) instead of relying on a measurement recorded in a golden JSON.

I took no NPU ticket: nothing here needs the device, and the owner's serve holds the lock on :18097.

bong-water-water-bong

VERDICT stays CHANGES, and this supersedes my earlier list: the zero-point nibble pairing this decoder implements is
not the file's, so every q4_1 tile it imports carries another row's zero point in half of its rows. The CodeQL item is
now the small one.

What the code does. decode_tile pairs the zero bytes into rows by halves —

const int n0 = r0 + 2 * cp;                                     // codes: byte cp -> rows 2cp, 2cp+1
...
const uint8_t z = s[zeros_off + i];
t.zp[size_t(r0 + i) ...] = z & 0x0F;                            // zero points: byte i -> rows i and i+nsubv/2
t.zp[size_t(r0 + nsubv / 2 + i) ...] = z >> 4;

— while the codes in the same subvolume use even/odd (n0 = r0 + 2*cp). One of the two has to be wrong, and it is the
zero points.

Measured, one file is enough. A correctly paired zero point sits where its 128 codes cluster, so per (row, 128-block)
mean(code) should track zp[row][block]. On the shipped pack's own bytes (~/lb/tprs-842/zp_pairing.py, prints all
four shapes):

                                  pairing     corr      |mean(code)-zp|
decode attn tile L0 (2,176)       halves   +0.0229        2.3787
decode attn tile L0 (2,176)       evenodd  +0.9970        0.1605
decode fused gate|up L0 (4,288)   halves   +0.0328        0.9861
decode fused gate|up L0 (4,288)   evenodd  +0.9621        0.2111
prefill gate L0 (2,176)           halves   +0.0620        1.0203
prefill gate L0 (2,176)           evenodd  +0.9665        0.2107
prefill down L0 (2,176)           halves   +0.0598        0.9468
prefill down L0 (2,176)           evenodd  +0.9613        0.2102

a shuffled zp gives corr ~0 and |diff| ~1.01, so the halves reading is indistinguishable from noise on every shape
while even/odd is ~0.96-0.997.

And the two physical layouts agree only under even/odd. Decoding the fused 4,288 gate|up group and the prefill file's
separate gate/up tensors (~/lb/tprs-842/crosscheck_prefill.py): codes identical 24/24 layers, scales identical 24/24,
and zero points identical 24/24 under even/odd, 0/24 under halves. The row correspondence itself is pinned by the
codes (every prefill gate row is found at fused row 0..31, 64..95, …, i.e. the 32-column interleave this PR claims), so
the zero-point result is not an alignment artefact.

Why nothing caught it: encode_tile mirrors decode_tile, so the 768/768 and 2160/2160 "survive decode→encode byte
for byte" checks are blind to a permutation that is applied twice; and both committed goldens
(oga_mlp_gu_q4_1.bin here, oga_attn_tile.bin from #829) were generated by this same decoder. The 32-byte field is
(col % Nsubv)/2 in FACTS §7.17 — byte i covers rows 2i and 2i+1, exactly what the codes do — while §7.13's
"rows 0..15 low, 16..31 high" is the reading the code inherited and the shipped bytes contradict.

Fix: pair the zero nibbles like the codes (r0 + 2*i, r0 + 2*i + 1) in decode_tile and encode_tile, then
re-generate both goldens (tools/oga_mlp_gu_golden.py, and tools/oga_attn_tile_golden.py for the attention one) — they
are currently fixtures for the bug, and the round-trip check will still pass afterwards, so the numbers to look at are the
container's tiles, not the round trip. Magnitude for triage: with a wrong zero point the row's weights are offset by
Δzp × scale, a few int4 steps of a 16-step range, so it is a wrong model rather than a crash — which is why the 291-tensor
import, the container read-back and every gate so far looked healthy.

Same defect, not this PR's doing: origin/main has it at npu/pack_import.cpp:622-624 (merged with the attention import
in #829), so the containers built since carry it for q/k/v/o as well. Fixing it here fixes both paths; the attention
goldens then need the same regeneration. I have filed it as an issue so it is not lost if this PR is rebased.

…he codes (closes #849) (#879) decode_tile()/encode_tile() read the zero-point nibbles of each 2,176-byte subvolume as row i / row i+16, while the codes of the same subvolume pair row 2i / 2i+1. The shipped pack's own bytes say even/odd: a correctly paired zero point sits where its 128 codes cluster, so per (row, K block) mean(code) must track zp -- +0.9977 under even/odd, -0.0227 under halves, on tile 2 of layer 0 taken straight out of the pack's .fconst. Every container imported since the attention payload landed (#829) therefore carried another row's zero point in half of its rows, for q/k/v/o. w = (code - zp) * scale, so the affected rows were offset by dzp x scale -- a few int4 steps. Silent: decode -> encode is invariant under a permutation applied on both sides, and the committed goldens were generated by this same decoder, so 768/768 attention tiles and 2160/2160 MLP groups still "passed". The fix is the two 2-line blocks. What comes with it: * tools/oga_attn_tile_golden.py carried the same halves reading (it is the independent numpy implementation the fixture is checked against, so it did not catch it either); re-paired, and tests/golden/oga_attn_tile_q4_1.bin regenerated from the pack. The raw tile fixture (oga_attn_tile.bin) is unchanged -- it is the vendor's bytes. * two assertions in tests/npu_pack_import_test.cpp, because the round trip cannot see this class: (a) a structural check on a tile whose nibbles are all distinct (byte i must give rows 2i/2i+1), and (b) the code-cluster statistic on the shipped tile (corr > 0.9). With the pairing reverted, the test fails on both plus the container golden; with the fix, all npu-pack checks pass. * a note in TileW's header comment so the MLP decoder on #842 does not repeat it. Co-authored-by: bong-water-water-bong <bong-water-water-bong@1bit.gg>

bong-water-water-bong

Verdict stays CHANGES, and the reason has changed shape since my 2026-11-02 review: main fixed half of it.

#879 (f3d9f6c0c, merged 10:45Z today) landed the zero-point pairing fix on main — decode_tile()/encode_tile() now read row 2i and 2i+1 from byte i ("The zero-point nibbles pair with rows exactly as the codes do (even/odd) … corr(zp, mean code) +0.998 vs −0.023 for halves"). This branch still carries the halves pairing:

$ gh pr diff 842 | grep -c 'r0 + 16 + i'
2

and that is exactly where it now conflicts (mergeable: CONFLICTING, untouched since 2026-11-02T19:27Z). So the MLP payload work itself is not what is blocking — the duplicate of a fix that already landed is.

To make this mergeable:

git fetch origin main && git rebase origin/main, and when the zp hunks conflict, take main's side: the pairing this branch implements is the one #849 was filed about.
Regenerate the oga_mlp_gu* goldens. They were produced with the halves reading, so they encode the wrong pairing; after the rebase they will fail against a correct decoder, and re-baselining them is the intended outcome, not a workaround.
Then #844 (1bit/671-oga-final-norm) can follow — it is stacked on this branch's head and would otherwise carry this PR's commits with it.

I re-verdict within minutes of the push — the checks are the golden regeneration plus npu_pack_import_test on the four OGA packs (Llama-3.2-1B, Qwen3-4B, Qwen3-1.7B, Qwen2.5-0.5B), which are all on this box.

The MLP section of a layer block was the last part of an OGA pack the importer refused: 72 projections on Qwen2.5-0.5B. It is now decoded, byte-verified and imported, and the remaining refusal is the final norm. Geometry, derived from the writer's own headers (mladfQuantMatrix / mladfWgtSubv in the wheel's ops_common/mladf_matmul_matrix.hpp, concat_gate_up in matmulbias_weights_gateup.cpp) and then MEASURED, not assumed: gate|up one fused hidden x 2*intermediate tensor, rows interleaved in 32-column chunks, in 128-column groups of 4,288-byte subvolumes (the same struct at Nsubv 64: 4,096 quants + 64 zeros + 64 bf16 scales) ...then one 64-byte parameter subvolume, the same bytes in every block down intermediate x hidden in 64-column groups of the 2,176-byte subvolume the attention payload already uses The three parts have to fill the block stride exactly, and that identity (not a constant) is what decides whether this pack is one the decoder understands: on Qwen2.5-0.5B they end at 8,708,288 B, the measured stride, to the byte. Qwen3-1.7B and Qwen3-4B do not add up and refuse by name with the size they did add up to. Decoder changes are the same code at a second width: TileW gains `rows` (64 or 128 columns), the subvolume size, its zero/scale offsets and its nibble index follow from it, and gu_half() splits a fused 128-column group into the two projections (gate = rows 0..31 and 64..95, up = 32..63 and 96..127). Verified, all of it on the shipped pack: * 2160/2160 MLP groups decode and re-encode to the pack's exact bytes (24 layers x 76 gate|up units + 24 x 14 down groups), the same contract the attention tiles are held to; * every weight in those groups agrees byte for byte with an INDEPENDENT decode of the same layer: the decode program stores gate/up fused at Nsubv 64 while the prefill program stores them separately at Nsubv 32, and the two agree on codes, scales and zero points for all 24 layers x {gate, up, down}; * a new CI fixture (tests/golden/oga_mlp_gu_group.bin, one 68,608-byte unit copied verbatim from the pack by tools/oga_mlp_gu_golden.py) pins the decode and the split against the vendor writer's own bytes with no vendor stack present, including the two controls that must fail; * container: 290 tensors / 67,088 q4_1 tiles (was 218 / 25,136), and the engine's own Model opens it. The bf16 parameter bound is unchanged and reported: zero points are bf16-exact for 52.9 % of the MLP's 32-column groups, 48.7 % of the attention ones.

…s class at 128 rows) The MLP section decodes 4,288-byte subvolumes, i.e. 64 rows per half instead of attention's 32, so #849's two assertions are repeated at that width: structural (distinct nibbles land on rows 2i / 2i+1) and statistical (corr(zp, mean code) on the pack's own unit, which is the only one of the two a wrong pairing cannot pass). The golden's numpy reference carried the halves reading, like the attention tool did, so it could not have caught this; re-paired, goldens regenerated from the pack.

bong-water-water-bong · 2026-11-03T18:32:32Z

Rebased onto main 65460184 and its CHANGES item is done, at 9be1d173 (branch force-pushed, ls-remote verified).

What changed, and why each part was needed:

Rebase takes main's side at the conflict — the two halves-pairing blocks are gone; the branch no longer carries a duplicate of npu-pack: pair the q4_1 zero-point nibbles with rows even/odd, like the codes (closes #849) #879's fix. What npu-pack: import the .fconst MLP section (gate|up fused, then down_proj) #842 keeps is its own generalization: group_bytes(k, rows), TileW::rows, and the 4,288-byte MLP subvolume. The pairing loops are now nsubv / 2 iterations over rows 2i / 2i + 1, which is main's even/odd rule at either width (32 rows per half in the attention payload, 64 in the fused gate|up unit).
Goldens regenerated from the pack — and tools/oga_mlp_gu_golden.py needed the same fix the attention tool did: it decoded zp as concatenate([z & 0xF, z >> 4]), i.e. halves. It is the "independent" numpy reference the fixture is checked against, so it could not catch the bug it shared. Re-paired to stack([z & 0xF, z >> 4], axis=-1), then oga_mlp_gu_group.bin / oga_mlp_gu_q4_1.bin / .json regenerated.
Two assertions added at the MLP width, because a decode→encode round trip is invariant under the pairing and cannot see it: structural (a subvolume whose nibbles are distinct must put them on rows 2i / 2i + 1 with 128 rows, 32 bytes per half) and statistical (corr(zp, mean code) > 0.9 on the pack's own unit — the one check a wrong pairing cannot pass).

That last one is the real evidence: even/odd scores above 0.9 on the MLP payload too, so #849's rule holds at the fused width, not just for attention.

Verification at 9be1d173: ctest -R "npu_elf_args|npu_design_grid|npu_forward_loader|npu_pack_import|npu_q4nx" 6/6 passed (including npu_pack_import_oga against the real 0.5B pack: all npu-pack checks passed), tools/copyright.py --check rc 0, all builds under ~/lb/thermal-run.sh. #844 follows this head; I am rebasing it next.

github-actions · 2026-11-03T18:32:43Z

Preparing review...

github-actions · 2026-11-03T18:38:33Z

Persistent review updated to latest commit bb3fffb

bong-water-water-bong

VERDICT: CHANGES at bb3fffbba — the pairing item I raised is fixed, but the pack leg regresses three of the four real OGA packs on this box, and this PR's own test is what says so. ctest 6/6 cannot see it, and that is the trap worth closing in this PR.

Measured, same binary, same packs, same invocation (npu_pack_import_test "" <pack>):

pack	main `ae0ee19`	this head `bb3fffbba`
`Qwen2.5-0.5B-Instruct_rai_1.8.0_npu_4K`	all checks passed	all checks passed
`Llama-3.2-1B-Instruct_rai_1.8.0_npu_4K`	all checks passed	3 check(s) failed
`Qwen3-1.7B_rai_1.8.0_npu_4K`	all checks passed	3 check(s) failed
`Qwen3-4B_rai_1.8.0_npu_4K`	all checks passed	3 check(s) failed

The three failures on each of the three packs:

FAIL pack: the MLP projections (refused only when their geometry does not add up) plus the final norm, got 113
FAIL pack: every refusal names where the bytes are and the fact that is missing
FAIL pack: the 4 attention projections per layer are planned from the measurement

The 113 is the informative one: the test expects layers*3 + 1 refusals when the MLP geometry does not add up (:654, 1B = 49), and this head produces 4*16 + 3*16 + 1 = 113, i.e. the four attention projections per layer are being refused as well — which is why attn_note.empty() == (attn_tiles > 0) (:661) also breaks. Some of those refusals additionally do not name fconst/layer block (:657-660). The 4B gives 361 = 36*10 + 1, so the per-layer count is not even constant across packs. ~/ogaload/oga8/c849, c849s and the 0.5B pack still pass, so whatever was validated is the small geometry.

Why ctest 6/6 is green: the pack leg is skipped unless a pack directory is passed — npu_pack_import_test "" <pack>; with no argument the binary prints skip pack: no pack directory given and the checks above never run (tests/npu_pack_import_test.cpp:840-851). So the self-test that gates CI exercises the crafted cases only. One way to close it for good: let the pack leg take the packs from an env list (as npu_elf_args_test does with its ELFs) and register a second add_test for the packs that exist, or drop the argument requirement and skip per pack.

What I confirmed as done: the zero-point pairing is now the even/odd reading #879 landed on main — npu/pack_import.cpp:690 iterates i < nsubv / 2 over "rows 2i, 2i+1, the pairing decode_tile() reads", identical to main's; npu_pack_import_test at this head reproduces my earlier fconst eps output for both epsilons; the two oga_mlp_gu* goldens are in the diff (regenerated, not carried); docs/npu.md documents the new section. The refusal wording nit is not a style point here — it is the second failing check.

Nit while you are in there: npu/pack_import.cpp:693 puts two statements on one line (for (…) { const uint16_t h16 = …).

bong-water-water-bong · 2026-11-03T18:46:52Z

Current with main again after #922 landed: merge commit 3d1b15da (no rewrite, so the branch history is append-only). Re-tested on that head: npu_pack_import_test tests/golden <0.5B pack> → all npu-pack checks passed (MLP golden 16/16 container tiles identical).

bong-water-water-bong

VERDICT: CHANGES at 3d1b15da2 — the head is Merge remote-tracking branch 'origin/main' on top of bb3fffbba, i.e. no code change, and the pack leg is red on the same three packs. A merge-only re-test cannot clear this, and "all npu-pack checks passed" is what the skipping invocation prints.

Reproduced here at this exact head (built from it, ninja npu_pack_import_test, then run):

$ ./npu_pack_import_test                                   # what ctest runs
skip pack: no pack directory given
all npu-pack checks passed                                 <- the green you saw

$ ./npu_pack_import_test "" /home/bcloud/models/oga/Llama-3.2-1B-Instruct_rai_1.8.0_npu_4K
3 check(s) failed

pack	main `ae0ee19`	`bb3fffbba`	`3d1b15da2`
Qwen2.5-0.5B-Instruct_rai_1.8.0_npu_4K	passed	passed	passed
Llama-3.2-1B-Instruct_rai_1.8.0_npu_4K	passed	3 failed	3 check(s) failed
Qwen3-1.7B_rai_1.8.0_npu_4K	passed	3 failed	3 check(s) failed
Qwen3-4B_rai_1.8.0_npu_4K	passed	3 failed	3 check(s) failed

The three, unchanged: the MLP projections … plus the final norm, got 113 (expected layers*3+1 = 49 at 16 layers, so four attention projections per layer are being refused too), every refusal names where the bytes are and the fact that is missing, the 4 attention projections per layer are planned from the measurement.

Why the shortcut is structural, not a lapse: CMakeLists.txt:596 registers add_test(NAME npu_pack_import COMMAND npu_pack_import_test ${CMAKE_SOURCE_DIR}/tests/golden) — the golden dir is argv[1], no pack is argv[2], so the pack leg self-skips and the binary still exits 0 with all npu-pack checks passed. The one test that does pass a real pack is optional: ONEBIT_OGA_PACK (:597-602), unset by default. So ctest, and any invocation without a pack path, will always print that line.

To clear it: fix the three packs (or scope the refusals so a pack without the MLP section refuses exactly layers*3+1 and keeps its attention tiles), then show the pack-leg run — the one command above is enough, and c849/c849s/0.5B passing is what a same-geometry check would report either way.

…leg as a pass (engine#842 review) The rebase onto main took this branch's simpler refusal accounting and dropped main's attention-aware one, so the three packs whose attention payload is not a whole number of tiles (Llama-3.2-1B, Qwen3-1.7B, Qwen3-4B) went red: 113/281/361 refusals where the branch expected layers*3+1. Restored main's per-class counts (norm / tail / MLP / attention / other) and kept the MLP expectation, so a pack without an MLP section refuses exactly layers*3+1 and keeps its attention tiles. Why it stayed invisible: ctest runs the test with the golden dir only and no pack, the pack leg self-skips, and the verdict line still said "all npu-pack checks passed". It now says whether the pack leg ran, and the skip is printed as a skip. Verified on all four real packs on this box (rc 0 each, "including the pack leg"): Llama-3.2-1B-Instruct, Qwen2.5-0.5B-Instruct, Qwen3-1.7B, Qwen3-4B; ctest 6/6; copyright rc 0.

github-actions · 2026-11-03T19:06:48Z

Preparing review...

Copilot

🟡 Changes recommended

Runtime MLP refusals can leave saved manifests incorrectly marking omitted projections as importable, while ineffective assertions and a removed epsilon-test invocation weaken regression coverage.

4 open issues

What changed in this PR

This PR extends the NPU pack importer to decode fused gate/up and down-projection MLP weights, leaving only the final norm refused on the supported Qwen2.5-0.5B pack.

Changes:

Generalizes tile decoding to support both attention and fused MLP widths, with byte-for-byte round-trip checks.
Adds MLP geometry validation, import reporting, and golden-fixture checks.
Documents the MLP layout, supported geometry, and updated import totals.

File	Description
`tools/oga_mlp_gu_golden.py`	Generates and checks MLP golden fixtures from a pack.
`tests/npu_pack_import_test.cpp`	Adds MLP decode, split, and import checks; clarifies skipped pack runs.
`tests/golden/oga_mlp_gu.json`	Records fixture provenance and fused gate/up layout.
`npu/pack_import.h`	Adds variable-width tile APIs and MLP planning/report fields.
`npu/pack_import.cpp`	Implements MLP geometry validation, decoding, splitting, and import.
`docs/npu.md`	Documents MLP support, layout, limitations, and import totals.

_{Review effort: Balanced}

bong-water-water-bong · 2026-11-03T21:07:59Z

Current at 08fc9b03 (CodeQL thread fix e83f3027 merged with main). Re-verified on this head, with the pack leg actually running all four packs:

pack	result
Llama-3.2-1B	rc=0 `all npu-pack checks passed (including the pack leg)`
Qwen2.5-0.5B	rc=0 `(including the pack leg)`
Qwen3-1.7B	rc=0 `(including the pack leg)`
Qwen3-4B	rc=0 `(including the pack leg)`

ctest -R "npu_pack_import|npu_q4nx|npu_forward_loader|npu_design_grid|npu_elf_args" 6/6, copyright.py --check rc 0.

bong-water-water-bong

VERDICT: READY at 08fc9b03e — Merge remote-tracking branch 'origin/main', pack leg green on all four real OGA packs at this head, and the CodeQL overflow thread is answered and resolved (CodeQL SUCCESS).

Qwen2.5-0.5B   rc=0  all npu-pack checks passed (including the pack leg)
Llama-3.2-1B   rc=0  all npu-pack checks passed (including the pack leg)
Qwen3-1.7B     rc=0  all npu-pack checks passed (including the pack leg)
Qwen3-4B       rc=0  all npu-pack checks passed (including the pack leg)

One thing this run turned up, and it is in the test, not the importer: tests/npu_pack_import_test.cpp:719 writes to a fixed path — fs::temp_directory_path() / "oga_pack_import_test" — and fs::remove_all()s it at :720 and :725. Two runs at the same time (another lane in the same pack family, or two arms of the same check) delete each other's directory mid-write, and the failure reads like a pack problem:

Llama-3.2-1B   rc=1  FAIL exception: filesystem error: cannot copy file: No such file or directory
                     […/Llama-3.2-1B-Instruct_rai_1.8.0_npu_4K/tokenizer.json] [/tmp/oga_pack_import_test/tokenizer.json]
Qwen3-1.7B     rc=1  FAIL exception: cannot write /tmp/oga_pack_import_test/model.q4nx
Qwen3-4B       rc=1  FAIL exception: cannot write /tmp/oga_pack_import_test/model.q4nx

I hit exactly that on my first pass at this head — two of four packs red, no FAIL pack: line anywhere — and the same four packs were green on a re-run, and green twice more in a row when nothing else was using the row. It is why my earlier "3 of 4 failed" and your "4 of 4" can both be honest for the same head. A per-process directory closes it:

const std::string out = (fs::temp_directory_path() /
    ("oga_pack_import_test-" + std::to_string(::getpid()))).string();

Not a merge blocker from me (the importer is fine; this is the row's own plumbing), but it is a false-FAIL source in a gate row that several lanes run concurrently tonight, and it is one line.

… controls (review) Four review threads on #842, all right: 1. The container's manifest came only from the plan, so a tensor the IMPORT refused at run time (a group that does not survive decode->encode, a nonempty bias subvolume, a missing tail envelope) was absent from the container while `incomplete` still listed only the planned refusals. Under --allow-incomplete that reported `incomplete: []` for weights that were skipped. The runtime refusals are now merged in: each is attributed to the tensor it names (`refusal` + `imported: false`) and appended to `incomplete`. 2. The even/odd structural checks assigned the accumulator inside the innermost loop, so an early wrong pair was hidden by a later right one and only the last pair of each K block was checked. Accumulated now -- in the MLP check and in the attention check, which has had the same defect since #849. 3. The gate/up swap control compared a gate-only tile stream against the gate+up one, so it passed on length alone. Both halves of the swapped unit are built before comparing. 4. #842's arrival dropped the only call of test_fconst_eps(), so neither ctest entry exercised the per-pack epsilon anchors any more. Restored. Also per-process scratch dir for the pack leg: two concurrent runs shared /tmp/oga_pack_import_test and one died with "cannot write .../model.q4nx" (measured today). Verified: end-to-end manifest proof with a pack whose MLP bias byte is non-zero -- the container it wrote lists the refusal in `incomplete` and marks mlp.gate_proj.weight imported=false with its reason; all four real packs pass the pack leg (rc 0); ctest 6/6; copyright rc 0.

github-actions · 2026-11-03T21:16:10Z

Persistent review updated to latest commit 2e9c5fa

…states, and test the tail envelope Four review threads on #844: 1. The prefill cross-check could abort an otherwise valid import: open_fconst() throws on an empty or unreadable prefill .fconst, before the "no readable tail envelope" note is reached. It is wrapped now -- a mapping failure leaves the cross-check unavailable and the import proceeds from the Token_Token copy. 2. `final_norm_crosscheck == false` conflated three different states, and the CLI reported "taken from the tail envelope (one copy to check)" for all of them -- including a norm that was REFUSED and never imported. The report now carries final_norm_imported and prefill_tail_seen, and the line reads "REFUSED (no readable tail envelope)" / "identical in both .fconst tails" / "taken from the Token_Token tail -- the two copies DISAGREE" / "taken from the tail envelope (the pack ships no prefill copy)". 3. The final-norm import had no coverage in the default CTest path (only where a real pack is passed, and three of the four packs here ship no prefill). New crafted leg test_final_norm_envelope(): it builds the smallest pack that reaches the tail envelope (8 layers so the locator's six-anchor floor is met), asserts the container carries exactly the bf16 bytes at tail+64, and then the control -- corrupt the tail epsilon, and the norm must be refused BY NAME, nothing may be written without --allow-incomplete, and the container that is written must list the omission. 4. The runtime-refusal manifest merge (from #842's review) applies here too: a refused final norm now appears in the container's `incomplete` list instead of leaving `incomplete: []`. Verified: crafted leg prints "final norm: crafted tail envelope imported from tail+64, missing-epsilon control refused"; all four real packs pass the pack leg (rc 0); ctest 2/2; copyright rc 0.

github-actions · 2026-11-04T00:31:11Z

Persistent review updated to latest commit 5bc07a3

bong-water-water-bong

VERDICT: READY at 2e9c5fa19 (the READY carries from 08fc9b03e; this commit is re-verified, pack leg included).

Re-ran the pack leg at this head (~/wt/tprs-842-build, cmake --build . --target npu_pack_import_test,
then ./npu_pack_import_test "" /home/bcloud/models/oga/<pack>):

Llama-3.2-1B-Instruct_rai_1.8.0_npu_4K   rc=0  all npu-pack checks passed (including the pack leg)
Qwen2.5-0.5B-Instruct_rai_1.8.0_npu_4K   rc=0  all npu-pack checks passed (including the pack leg)
Qwen3-1.7B_rai_1.8.0_npu_4K              rc=0  all npu-pack checks passed (including the pack leg)
Qwen3-4B_rai_1.8.0_npu_4K                rc=0  all npu-pack checks passed (including the pack leg)

The three test controls are real fixes, and the first one was hiding a hole. pairs_even_odd = pairs_even_odd && … — the old assignment left only the last i of each block checked, so the 0.5B/4B attention-tile and MLP zero-point
pairing gates could pass with every earlier pair wrong. The gate/up swap control now tiles both streams
(for (const bool up : {false, true})) and compares sizes, so it can no longer "catch" a length difference as a swap.
The per-pid scratch dir is the shared-/tmp flake I reported (two concurrent runs deleting each other's
/tmp/oga_pack_import_test); ("oga_pack_import_test." + std::to_string(::getpid())) closes it.

Manifest merge — read, and the string contract checks out. p.refused() is built as t.name + ": " + t.refusal
(npu/pack_import.cpp:809), which is the ": " split the merge relies on for per-tensor attribution; the one
header-level refusal in npu/flm_head_import.cpp:143 has no name, and the merge's c == std::string::npos guard
correctly still lists it in incomplete without pretending to attribute it. w.finish(json::parse(manifest))
re-parsing the plan's JSON is fine.

One coverage note, not a gate: the new assertions did not execute in my four-pack run. They sit inside
if (!r.refused.empty()) (tests/npu_pack_import_test.cpp:734) and none of the four real packs refuses anything
(grep -c refused on all four logs: 0), so "lists every runtime refusal" / "imported=false" are unexercised here —
they only run for a pack that refuses by name. The golden leg already crafts refusals (crafted embed refusal: model.embed_tokens.weight: needs 4194304 B, declares 1024 in model.onnx.data), so a --allow-incomplete import of
that same crafted pack is the missing case.

For what it is worth, truncating a pack's model.onnx.data is not a way to produce a refusal: at 200 MB of 334 MB
the import throws FAIL exception: model.embed_tokens.weight: external data past the end of model.onnx.data, i.e.
it fails before the per-tensor refusal path. (I did that on a reflinked copy with a distinct inode — the shared pack
is untouched.)

CI: build + Analyze x4 + CodeQL SUCCESS, pr_agent cancelled by a superseding run, hw_gate SKIPPED. Merge order:
this before #844 (which carries it), as before.

…rown out of the importer read_declared() iterated <pack>/cache directly, so a pack without that directory threw std::filesystem_error out of the import and the harness printed FAIL exception: filesystem error: directory iterator cannot open directory: .../cache for the *hybrid* flavour of these packs, which ships model.bin instead of dd_metastate_*.fconst and has no cache/ at all -- a different on-disk layout, not a broken one. Two changes, both about saying what happened: * read_declared(): a missing cache/ means no declared N/K, which is what an empty map already says; the absence is reported once on stderr instead of throwing. * the harness: a pack with no dd_metastate_* is recognised as the hybrid flavour, named, and the pack leg is skipped -- the verdict line says "pack leg SKIPPED, not an OGA NPU pack" rather than claiming a pass it did not earn. Verified on the five packs on this box: the four real OGA packs still run the pack leg and pass rc=0 (Llama-3.2-1B, Qwen2.5-0.5B, Qwen3-1.7B, Qwen3-4B), and the hybrid pack now prints the skip and exits 0 instead of failing with a filesystem exception.

bong-water-water-bong · 2026-11-04T01:43:57Z

New commit 49baaee8: a pack with no compiler metadata is now skipped by name instead of throwing out of the importer.

Found by running the pack leg on all five pack directories on this box rather than the four I had been using. The fifth, Qwen2.5-0.5B-Instruct_rai_1.8.0_hybrid, is the model.bin flavour of these packs: it has no dd_metastate_*.fconst and no cache/, and read_declared() iterated <pack>/cache unguarded, so it surfaced as

FAIL exception: filesystem error: directory iterator cannot open directory: .../cache

which says nothing about what the pack is.

read_declared(): a missing cache/ means no declared N/K — which is what an empty map already says — and the absence is reported once on stderr.
the harness: a pack with no dd_metastate_* is recognised as the hybrid flavour, named, and the pack leg is skipped, with the verdict line saying pack leg SKIPPED, not an OGA NPU pack rather than claiming a pass it did not earn (the same failure mode as the earlier "green line that hid it").

Re-verified on this head, all five directories:

pack	before	after
Llama-3.2-1B	rc=0, pack leg ran	rc=0, pack leg ran
Qwen2.5-0.5B	rc=0, pack leg ran	rc=0, pack leg ran
Qwen3-1.7B	rc=0, pack leg ran	rc=0, pack leg ran
Qwen3-4B	rc=0, pack leg ran	rc=0, pack leg ran
Qwen2.5-0.5B hybrid	rc=1, `FAIL exception: filesystem error…`	rc=0, `skip pack: … hybrid/model.bin flavour`

tools/copyright.py --check rc 0.

For the record, my first check of this pack was a false green: I read rc=$? after a pipe through tail, so it was tail's status. The loop above captures the exit code before piping.

github-actions · 2026-11-04T01:44:06Z

Preparing review...

github-actions · 2026-11-04T01:49:25Z

Persistent review updated to latest commit 49baaee

bong-water-water-bong

VERDICT: READY at 49baaee85. Re-ran the whole five-pack sweep at this head and the new skip behaves exactly as
described — it names what it is instead of throwing, and the final line no longer reads as a pass.

Llama-3.2-1B-Instruct_rai_1.8.0_npu_4K    rc=0  all npu-pack checks passed (including the pack leg)
Qwen2.5-0.5B-Instruct_rai_1.8.0_npu_4K    rc=0  all npu-pack checks passed (including the pack leg)
Qwen2.5-0.5B-Instruct_rai_1.8.0_hybrid    rc=0  skip pack: … ships no dd_metastate_*.fconst, so it is the hybrid/model.bin
                                                flavour of these packs and not an OGA NPU pack this importer reads
                                                all npu-pack checks passed -- pack leg SKIPPED, not an OGA NPU pack
Qwen3-1.7B_rai_1.8.0_npu_4K               rc=0  all npu-pack checks passed (including the pack leg)
Qwen3-4B_rai_1.8.0_npu_4K                 rc=0  all npu-pack checks passed (including the pack leg)

That is the shape of fix I asked for two reviews ago: four legs that actually run, one that says which pack it is and
why it is not being read, and a verdict line that cannot be mistaken for the four. read_declared() now returns an
empty map with a note instead of letting std::filesystem_error escape the importer, and the directory_iterator
also takes the error_code, so a mid-iteration failure cannot throw either.

Two notes, neither a gate:

read_declared() reports "has no cache/" for any reason is_directory fails — a pack with a cache/ that cannot
be opened (permissions, I/O) gets the same sentence and the same skipped cross-checks. The fallback is the safe
direction (fewer checks, loudly noted), but the message could carry ec.message() when ec is set so the two
cases read differently.
Consequence worth knowing for whoever imports one directly: the harness now skips the hybrid pack entirely, so
the new "no cache ⇒ empty declarations" path inside import_pack is not exercised by any leg. 1bit npu-pack import on that pack is the only way to reach it; if that flavour is meant to be supported, a crafted
no-cache/ directory in the golden leg would cover it the way the crafted refusals cover the rest.

Everything I verified at 2e9c5fa19 (the manifest merge, the three fixed test controls, the per-pid scratch dir)
is unchanged by this commit, and the merge of main (5bc07a3) brings #934/#938/etc without touching the importer.
CI: build + Analyze ×4 + CodeQL green at this head.

…ed /tmp path pi-prs' note 2 on #936: key_on and key_off both redirected to /tmp/1bit-mint-selftest.log, so a vm-check row and a person running the selftest at the same time would interleave in one file - the same class as the /tmp/oga_pack_import_test flake on #842. $d/mint.log costs nothing and cannot collide. Measured: --mint-selftest still rc=0 (mint 1 line / 48 hex / 0600 / other lines kept; off: 0 lines, clean), and the scratch dir now holds lemond.conf + mint.log, nothing outside it.

…w (follow-up to #935's review notes 2 and 5) (#936) * os/v3/puppy: the mint path gets a selftest and a gate row, and "shown" keeps its shape guard Both are notes pi-prs left on #934 (items 2 and 5), both real: 2. Item 1's write path - the only new write in this image - could only be reached from a dialog, with CONF and the restart hard-coded, so nothing could drive it at a temp path. Now CONF=${ONEBIT_LEMOND_CONF:-/etc/1bit/lemond.conf} and LEMOND_INIT=${ONEBIT_LEMOND_INIT:-/etc/init.d/10lemonade} (defaults unchanged; a boot sets neither), and `1bit-console-menu --mint-selftest <dir>` drives key_on/key_off there with the restart replaced by `true`: asserts one 48-hex line, mode 600, the other lines kept, then no line and no <conf>.new left. A new vm-check row `U3 key toggle` runs it INSIDE the image, so the mint is a gate row rather than a manual test (the reason it is stacked behind #934: it changes an embedded file, so it needs its own image). 5. The read-only half printed `api-key-shown` for any non-empty value. A hand-edited or glued line can hold a fragment, so the marker is back behind the >=32-character guard while the key itself is still printed (release-check and menugate read the value, not the marker). Measured here: --mint-selftest rc=0 (`mint: lines=1 hex=48 mode=600 other-lines-kept=1` / `off: lines=0 clean` / `mint-selftest-ok`), and the first run of it failed on `|| echo 0` making grep's count "0\n0" - fixed and noted in the file. sh -n / bash -n / embedded-python compile / copyright rc=0, shellcheck adds nothing. * os/v3/puppy: the mint selftest logs inside its scratch dir, not a fixed /tmp path pi-prs' note 2 on #936: key_on and key_off both redirected to /tmp/1bit-mint-selftest.log, so a vm-check row and a person running the selftest at the same time would interleave in one file - the same class as the /tmp/oga_pack_import_test flake on #842. $d/mint.log costs nothing and cannot collide. Measured: --mint-selftest still rc=0 (mint 1 line / 48 hex / 0600 / other lines kept; off: 0 lines, clean), and the scratch dir now holds lemond.conf + mint.log, nothing outside it. --------- Co-authored-by: pi-os3 <pi-os3@1bit.local> Co-authored-by: bong-water-water-bong <bong-water-water-bong@1bit.gg>

github-actions · 2026-11-04T06:26:12Z

Persistent review updated to latest commit e2ce4d5

github-actions · 2026-11-04T06:30:44Z

Persistent review updated to latest commit 2cd33d4

github-actions · 2026-11-04T06:46:20Z

Persistent review updated to latest commit 39a3a81

github-actions · 2026-11-04T06:55:26Z

Persistent review updated to latest commit c4d8dfc

github-actions · 2026-11-04T06:59:35Z

Preparing review...

bong-water-water-bong added enhancement New feature or request Review effort 4/5 labels Nov 2, 2026

github-advanced-security Bot found potential problems Nov 2, 2026

View reviewed changes

Comment thread tests/npu_pack_import_test.cpp Fixed

bong-water-water-bong commented Nov 2, 2026

View reviewed changes

This was referenced Nov 2, 2026

npu-pack: import the final norm from the .fconst tail envelope (the pack now converts whole) #844

Merged

npu: the forward derives GEMM K from hidden, but a Q4NX container pads K to the 256-column grid (hidden 896 -> 1024) #845

Closed

bong-water-water-bong commented Nov 2, 2026

View reviewed changes

bong-water-water-bong commented Nov 3, 2026

View reviewed changes

bong-water-water-bong added 2 commits November 3, 2026 13:46

bong-water-water-bong force-pushed the 1bit/671-oga-mlp-payload branch from c84110a to 9be1d17 Compare November 3, 2026 18:32

Merge remote-tracking branch 'origin/main' into HEAD

bb3fffb

bong-water-water-bong commented Nov 3, 2026

View reviewed changes

Merge remote-tracking branch 'origin/main' into HEAD

3d1b15d

bong-water-water-bong commented Nov 3, 2026

View reviewed changes

bong-water-water-bong force-pushed the 1bit/671-oga-mlp-payload branch from 7f5d2ea to a4afb54 Compare November 3, 2026 19:06

Merge remote-tracking branch 'origin/main' into 1bit/671-oga-mlp-payload

08fc9b0

Copilot AI reviewed Nov 3, 2026

View reviewed changes

Comment thread npu/pack_import.cpp Resolved

Comment thread tests/npu_pack_import_test.cpp Resolved

Comment thread tests/npu_pack_import_test.cpp Resolved

Comment thread tests/npu_pack_import_test.cpp Resolved

bong-water-water-bong commented Nov 3, 2026

View reviewed changes

Merge remote-tracking branch 'origin/main' into 1bit/671-oga-mlp-payload

5bc07a3

bong-water-water-bong commented Nov 4, 2026

View reviewed changes

bong-water-water-bong mentioned this pull request Nov 4, 2026

os/v3/puppy: drive the API-key mint from a selftest and a vm-check row (follow-up to #935's review notes 2 and 5) #936

Merged

bong-water-water-bong commented Nov 4, 2026

View reviewed changes

Merge branch 'main' into 1bit/671-oga-mlp-payload

e2ce4d5

Merge branch 'main' into 1bit/671-oga-mlp-payload

2cd33d4

Merge branch 'main' into 1bit/671-oga-mlp-payload

39a3a81

Merge branch 'main' into 1bit/671-oga-mlp-payload

c4d8dfc

Merge branch 'main' into 1bit/671-oga-mlp-payload

589e397

bong-water-water-bong merged commit ea067b6 into main Nov 4, 2026
8 of 9 checks passed

bong-water-water-bong deleted the 1bit/671-oga-mlp-payload branch November 4, 2026 07:01

Conversation

bong-water-water-bong commented Nov 2, 2026

What

The layout, and why it is a measurement

Code

Verified on the shipped pack

Checks run

Not in this PR

Uh oh!

github-actions Bot commented Nov 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Reviewer Guide 🔍

(Review updated until commit c4d8dfc)

Uh oh!

Uh oh!

bong-water-water-bong left a comment

Choose a reason for hiding this comment

Uh oh!

bong-water-water-bong left a comment

Choose a reason for hiding this comment

Uh oh!

bong-water-water-bong left a comment

Choose a reason for hiding this comment

Uh oh!

bong-water-water-bong commented Nov 3, 2026

Uh oh!

github-actions Bot commented Nov 3, 2026

Uh oh!

github-actions Bot commented Nov 3, 2026

Uh oh!

bong-water-water-bong left a comment

Choose a reason for hiding this comment

Uh oh!

bong-water-water-bong commented Nov 3, 2026

Uh oh!

bong-water-water-bong left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented Nov 3, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

bong-water-water-bong commented Nov 3, 2026

Uh oh!

bong-water-water-bong left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented Nov 3, 2026

Uh oh!

github-actions Bot commented Nov 4, 2026

Uh oh!

bong-water-water-bong left a comment

Choose a reason for hiding this comment

Uh oh!

bong-water-water-bong commented Nov 4, 2026

Uh oh!

github-actions Bot commented Nov 4, 2026

Uh oh!

github-actions Bot commented Nov 4, 2026

Uh oh!

bong-water-water-bong left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented Nov 4, 2026

Uh oh!

github-actions Bot commented Nov 4, 2026

Uh oh!

github-actions Bot commented Nov 4, 2026

Uh oh!

github-actions Bot commented Nov 4, 2026

Uh oh!

github-actions Bot commented Nov 4, 2026

Uh oh!

Uh oh!

github-actions Bot commented Nov 2, 2026 •

edited

Loading

(Review updated until commit `c4d8dfc`)